-
Notifications
You must be signed in to change notification settings - Fork 8k
drivers: gpio: intel: Enable support for new ACPI GINF method #95920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
684d7fc
to
8812842
Compare
drivers/gpio/Kconfig.intel
Outdated
as it is observed that the offset varies from | ||
platform to platform. | ||
|
||
config GPIO_INTEL_INT_STAT_OFFSET |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if these two actually belong to devicetree, as they are per platform (so they would be added to dts/bindings/gpio/intel,gpio.yaml
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, adding it as a dts parameter is more suitable but the value is common for all the GPIO instances of a platform. That's why, it is added as a CONFIG to be updated once for a platform instead of adding same value for all the instances in dts file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... not sure how this is usually (supposed to be) handled. @henrikbrixandersen thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically, we would describe that in the devicetree instance, but there's no good way of setting the same value across a number of devicetree nodes. Only thing I can think of is having a parent node with this property, but that would be kind of superficial.
Is this GPIO controller supported in Linux? if yes, how does the Linux kernel handle/provide this offset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This GPIO controller is supported in Linux, a different ACPI method is used to fetch this offset directly instead of deriving from another offset like what we are doing hereLinux Ref. That ACPI method is not enabled in zephyr.
dts/bindings/gpio/intel,gpio.yaml
Outdated
type: int | ||
description: Pin offset of this GPIO entry | ||
|
||
acpi-ginf: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not familiar with ACPI at all, but it seems weird that a boolean describes a version. Maybe rename it to acpi-ginf-3-param
or so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will update it to acpi-ginf-3-param
acpi_child = acpi_device_get(hid, uid); | ||
if (!acpi_child) { | ||
printk("acpi_device_get failed\n"); | ||
LOG_ERR("acpi_device_get failed"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: maybe have these s/printk/LOG_ERR
on a different patch, so it doesn't distract from the functional changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall I add this change in a different comment in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be ideal, yes (assuming you meant "commit").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, commit
@akanisetti will you update this PR? |
Hi @edersondisouza, I am waiting for @henrikbrixandersen's response regarding #95920 (comment). |
I wasn't aware you were waiting for a response from me, my apologies. I don't think I have a good solution on how to obtain this offset in Zephyr. |
No worries @henrikbrixandersen. |
I would prefer a devicetree-based solution for specifying the offset. |
Thank you @henrikbrixandersen, |
Enable support for ACPI_RESOURCE_TYPE_ADDRESS64 in acpi_device_mmio_get to fetch 64bit address from resource of an ACPI device. Signed-off-by: Anisetti Avinash Krishna <[email protected]>
8812842
to
9dd44e2
Compare
Enable support for latest GINF method which requires 3 paramters for each GPIO group and enables gpio support for intel_ptl_h platform. Signed-off-by: Anisetti Avinash Krishna <[email protected]>
Replace printk with LOG_ERR by adding a log module. Signed-off-by: Anisetti Avinash Krishna <[email protected]>
9dd44e2
to
2e4aaa6
Compare
|
This PR adds GPIO support for Intel Panther Lake H SoC, implementing a new ACPI GINF method for GPIO resource enumeration and adding corresponding device tree definitions.
Adds GPIO device tree definitions for Panther Lake H with new ACPI GINF method support
Implements enhanced ACPI GPIO resource enumeration with support for both old and new GINF methods
Adds GPIO configuration overlay for test board intel_ptl_h_crb